feat: limit in-flight operations in map/parallel#533
Draft
yaythomas wants to merge 1 commit into
Draft
Conversation
9f04c8d to
3aa2bdc
Compare
3aa2bdc to
0940152
Compare
max_concurrency previously sized the thread pool only, so branches that suspended (invoke, wait, callbacks) released their thread and the next item started immediately. All items were initiated at once regardless of the configured limit. Rework the concurrency executor around a single coordinator loop: - The calling thread owns all branch state and schedules work; worker threads only run branches and report outcomes on a queue. No locks remain in the module. - max_concurrency now bounds in-flight branches. A suspended branch keeps its slot until it reaches a terminal state, matching the JS and Java SDKs. When every in-flight branch is suspended, the parent suspends with the earliest resume timestamp, or indefinitely. - Branches start in index order and operation ids derive from the branch index, so scheduling stays deterministic across invocations. - Timed suspends resume in-process via the event queue timeout, replacing the TimerScheduler background thread. - Consolidate completion logic (thresholds and reason inference) into CompletionPolicy, replacing ExecutionCounters and the duplicated logic in BatchResult. - Omit never-started branches from BatchResult, matching the JS SDK. - Stop scheduling when a branch is orphaned: an ancestor completed, so all further checkpoints under it would be rejected. - Record the completion decision in map/parallel summaries and obey it on replay: summaries carry the started-branch index set (or its complement, whichever is smaller) and replay reconstructs the exact live result instead of re-deriving statuses from child checkpoints. Summaries without the record fall back to checkpoint derivation. This also reconstructs FLAT results on replay, which checkpoint derivation could not discriminate. - Validate config bounds at construction, matching the Java SDK: max_concurrency and min_successful must be at least 1, tolerated_failure_count non-negative, tolerated_failure_percentage within 0-100. Previously max_concurrency=0 silently meant unlimited and min_successful=0 silently meant all items. Closes #279
0940152 to
69100b1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
max_concurrencypreviously sized the thread pool only, so branches that suspended (invoke, wait, callbacks) released their thread and the next item started immediately. All items were initiated at once regardless of the configured limit.Rework the concurrency executor around a single coordinator loop:
max_concurrencynow bounds in-flight branches. A suspended branch keeps its slot until it reaches a terminal state, matching the JS and Java SDKs. When every in-flight branch is suspended, the parent suspends with the earliest resume timestamp, or indefinitely.TimerSchedulerbackground thread.CompletionPolicy, replacingExecutionCountersand the duplicated logic inBatchResult.completionReasonplus the started-branch index set (startedIndexes, orcompletedIndexeswhen smaller), and replay reconstructs the exact live result — same items, same statuses, same reason — instead of re-deriving from child checkpoints. Summaries without the record (checkpoints written by earlier SDK versions, custom summary generators) fall back to checkpoint derivation, preserving old behavior. This also makes FLAT-nesting results reconstructable on replay, which checkpoint derivation cannot discriminate. Handlers now default the summary generator per-field, so user-provided configs get exact replay without opting in.Behavior changes
BatchResultomits never-started branches (matching the JS SDK). Previously every item appeared as STARTED even when an early completion meant it never ran;total_countnow reflects branches that actually started.min_successfulset no longer fail fast on the first failure. The old tolerance check ignoredmin_successfulwhen deciding whether criteria existed, so a single failure stopped scheduling; scheduling now continues past failures untilmin_successfulis reached or all items finish, matching the JS and Java SDKs.max_concurrencyandmin_successfulmust be at least 1,tolerated_failure_countnon-negative,tolerated_failure_percentagewithin 0–100. Previouslymax_concurrency=0silently meant unlimited andmin_successful=0silently meant all items.Testing
max_concurrencysemantics and asuccess_countundercount whenmin_successfulwas not reached.Issue #, if available:
Closes #279
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.